-
Notifications
You must be signed in to change notification settings - Fork 912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor shell rules #301
Merged
Merged
Refactor shell rules #301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactoring the shell related rules to avoid FPs. Instead of considering all shells suspicious and trying to carve out exceptions for the legitimate uses of shells, only consider shells spawned below certain processes suspicious. The set of processes is a collection of commonly used web servers, databases, nosql document stores, mail programs, message queues, process monitors, application servers, etc. Also, runsv is also considered a top level process that denotes a service. This allows a way for more flexible servers like ad-hoc nodejs express apps, etc to denote themselves as a full server process.
spawn_shell is now a silent action. its replacement is spawn_shell_under_httpd, which respawns itself as httpd and then runs a shell. db_program_spawn_binaries now runs ls instead of a shell so it only matches db_program_spawn_process.
Start the express server using runit's runsv, which allows falco to consider any shells run by it as suspicious.
In draios/sysdig#757 the path argument for mkdir moved to the second argument. This only became visible in the unit tests once the trace files were updated to reflect the other shell rule changes--the trace files had the old format.
Shell in container doesn't exist any longer and its functionality has been subsumed by run shell untrusted.
mstemm
force-pushed
the
refactor-shell-rules
branch
from
November 17, 2017 23:19
ed3b7fd
to
6f610ed
Compare
In some cases, these are run below a service runsv so we still need exceptions for them.
mstemm
force-pushed
the
refactor-shell-rules
branch
from
November 20, 2017 19:11
0bbed74
to
c5b1f4c
Compare
There's enough evidence of people spawning general commands that we can't protect it.
mstemm
force-pushed
the
refactor-shell-rules
branch
from
November 20, 2017 19:32
df58394
to
5a85cf1
Compare
Move the nginx exception to the main rule instead of the protected_shell_spawner macro. Also add erl_child_setup (related to rabbitmq) as an allowed shell spawner.
All off these are either below nginx, httpd, or runsv but should still be allowed to spawn shells.
mstemm
force-pushed
the
refactor-shell-rules
branch
from
November 22, 2017 19:36
6015a84
to
e89dd01
Compare
Skip shells when any process ancestor (parent, gparent, etc) is a package management binary. This includes the program needrestart. This is a deep search but should prevent a lot of other more detailed exceptions trying to find the specific scripts run as a part of installations.
Serf is a service discovery tool and can in some cases be spawned by apache/nginx. Also allow shells that are just checking the status of pids via kill -0.
Add several exclusions back from the shell in container rule. These are all allowed shell spawns that happen to be below nginx/fluentd/apache/etc.
This saves space as well as cleanup. I haven't yet removed the macros/lists used by these rules and not used anywhere else. I'll do that cleanup in a separate step.
Add back the exclusions based on command lines, using the existing set of command lines.
Of note is runsv, which means it can directly run shells (the ./run and ./finish scripts), but the things it runs can not.
We'll detect the first shell and not any other shells it spawns.
mstemm
force-pushed
the
refactor-shell-rules
branch
from
November 27, 2017 21:48
ead8662
to
a0786c7
Compare
In some cases, the initial process for a container can have a parent "runc:[0:PARENT]", so also allow those cases to count as a container entrypoint.
Use the container_entrypoint macro to denote entering a container and also allow exe to be one of the processes that's the parent of an entrypoint.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the shell-related rules to reduce false positives. These changes significantly decrease the scope of the rules so they trigger only for shells spawned below specific processes instead of anywhere. See the commit messages for more details.